[DIT-10076] Unit tests for API Token service methods #118
Conversation
| ], | ||
| watchPathIgnorePatterns: ["<rootDir>/.testing/", "<rootDir>/testing/"], | ||
| collectCoverageFrom: ["lib/**/*.{js,jsx,ts,tsx}"], | ||
| restoreMocks: true, |
There was a problem hiding this comment.
In general, I still have calls in each test file for this, but you can set this here to automatically restore. Seemed like a nice catch-all in case we forget somewhere.
| priorToken = appContext.apiToken; | ||
| priorHost = appContext.apiHost; | ||
| appContext.setApiToken(""); | ||
| appContext.apiHost = apiHost; |
There was a problem hiding this comment.
For the functions that use appContext values that we want to specify, i'm saving the original value, changing it to what we want, then restoring the prior value after the test. This might be total overkill, but it seemed like a nice safety measure. Thoughts?
There was a problem hiding this comment.
I think this is fine for now.
| expect(logger.url).toHaveBeenCalled(); | ||
| expect(logger.bold).toHaveBeenCalled(); | ||
| expect(logger.info).toHaveBeenCalled(); | ||
| expect(logger.writeLine).toHaveBeenCalled(); |
There was a problem hiding this comment.
Is this overkill? Should i ditch the logger checks? Does it make the test too brittle/resistent to change?
There was a problem hiding this comment.
I think it's okay to be brittle here as this will rarely change, and it does help us know that the messages were logged out. if the messages weren't getting logged out, that would be useful to know.
| import collectAndSaveToken from "./collectAndSaveToken"; | ||
| import validateToken from "./validateToken"; | ||
| import getURLHostname from "./getURLHostname"; | ||
|
|
There was a problem hiding this comment.
This method was updated to remove the optional parameters (never used in the real code) and hard-code to the appContext values. Would be a good idea to sanity check me here.
There was a problem hiding this comment.
it's fine to remove the optional parameters, it should all still work the same.
| return result.output?.join("\n") || "Invalid API key"; | ||
| } | ||
| return true; | ||
| }; |
There was a problem hiding this comment.
I moved the validate fn to its own saved function so it could be tested. It's identical to what was previously defined inline on line 22.
jaerod95
left a comment
jaerod95
left a comment
There was a problem hiding this comment.
Tested the branching logic for the api token service and everything seems to be in working order!
Overview
Writes unit tests for each of the api token service methods.
Since these methods call each other, I ran into to issues with proper mocking when they were all in one file, so I reorganized to put each method in its own file, and created a test file for each.
Generally speaking, each method was just copy/pasted into the new file, but I did make a couple updates:
initAPIToken- they were only added for the purpose of testing and it turns out I didn't need themvalidatemethod we pass intopromptto a named function so it could be testedContext
https://linear.app/dittowords/issue/DIT-10076/add-tests-for-apitoken-service
Test Plan
Testing successfully completed in via:
npx jest apiToken(to run all the tests in the new directory)